Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[MM] Change to the common resource manager #567

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

again4you
Copy link
Collaborator

This patch updates the mm resource manager.

Change-Id: I108b500c796a8899e6d582346a184d9e6a8136c4

Signed-off-by: YoungHun Kim [email protected]
Signed-off-by: Sangjung Woo [email protected]

This patch updates the mm resource manager.
- Origin: https://review.tizen.org/gerrit/#/c/platform/core/api/machine-learning/+/318887/

Change-Id: I108b500c796a8899e6d582346a184d9e6a8136c4

Signed-off-by: YoungHun Kim <[email protected]>
Signed-off-by: Sangjung Woo <[email protected]>
@taos-ci
Copy link
Collaborator

taos-ci commented Oct 11, 2024

📝 TAOS-CI Version: 1.5.20200925. Thank you for submitting PR #567. Please a submit 1commit/1PR (one commit per one PR) policy to get comments quickly from reviewers. Your PR must pass all verificiation processes of cibot before starting a review process from reviewers. If you are new member to join this project, please read manuals in documentation folder and wiki page. In order to monitor a progress status of your PR in more detail, visit http://ci.nnstreamer.ai/.

@@ -76,7 +76,7 @@ Summary: Tizen native API for NNStreamer
# 2. Tizen : ./packaging/machine-learning-api.spec
# 3. Meson : ./meson.build
# 4. Android : ./java/android/nnstreamer/src/main/jni/Android.mk
Version: 1.8.6
Version: 1.1.0
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The rpm version is changed! why?

}
status = ml_tizen_mm_res_allocate(p->resources, res_type);
if (status != ML_ERROR_NONE) {
_ml_loge("Faied to allocate resource.");
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  • Faied -> Failed

for (type = RES_TYPE_VIDEO_DECODER; type < RES_TYPE_MAX; type++) {
ret = ml_tizen_mm_res_deallocate(mm_handle, type);
if (ret != ML_ERROR_NONE) {
_ml_loge("Fail to deallocate resoure [type %d].", type);
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  • resoure -> resource

@@ -23,7 +23,7 @@ if (get_option('enable-tizen'))

nns_capi_deps += dependency('mm-camcorder')
if (tizenVmajor >= 5)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
if (tizenVmajor >= 5)
if (tizenVmajor >= 9)

@@ -110,7 +110,8 @@ BuildRequires: pkgconfig(capi-privacy-privilege-manager)
%endif
BuildRequires: pkgconfig(mm-camcorder)
%if 0%{tizen_version_major} >= 5
Copy link
Member

@anyj0527 anyj0527 Oct 11, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

5 -> 9. Or make an another if phrase ?

base = g_path_get_basename(contents);

if (g_strlcpy(appid, base, size) >= size)
LOGE("string truncated");
Copy link
Member

@gichan-jang gichan-jang Oct 11, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Return error or change log level to warning?

_ml_logi("app id : %s type %d", resources->rci.app_id, type);
memset(&request_resources, 0x0, sizeof(rm_category_request_s));

device = &resources->devices[type];
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Isn't validation of device necessary?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants